better base check (json and human) #11604
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the but base check command by implementing proper JSON output support (previously a no-op), correcting the display to show upstream commits instead of recent commits, and updating the output styling to use simpler ANSI colors instead of emojis.
Key Changes:
- Implemented functional
--jsonflag with structured output including base branch info, upstream commits, and branch statuses - Fixed commit display to show
upstream_commitsrather thanrecent_commits - Modernized human-readable output with ANSI color styling (dimmed, colored status indicators) replacing emoji-heavy format
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
There is just one comment from me, and maybe the request for a test as I don't think there is one yet. Pinning the output now to something you like will help to keep it that way later once the command is 'modernised'.
| /// JSON output for `but base check` | ||
| #[derive(Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| struct BaseCheckOutput { |
There was a problem hiding this comment.
All these would be better when placed in a json module (I think it's also done in but status).
There was a problem hiding this comment.
@Byron split this into base/mod.rs and base/json.rs
Provide a machine-readable JSON representation of the `but base check` command so callers can programmatically consume base branch info, upstream commits and branch mergeability status. Introduce serializable structs (BaseCheckOutput, BaseBranchInfo, UpstreamInfo, UpstreamCommit, BranchStatusInfo), fetch the data before deciding output mode, and serialize the gathered data (base branch details, upstream commit list and count, branch statuses, up_to_date and worktree conflict flags). The human-readable output path is preserved.
Fix the JSON output for the "but base check" command to report only actual upstream commits. The code previously used recent_commits (which lists recent commits regardless of whether they're new upstream commits), causing the reported count to mismatch the listed commits. Switched to upstream_commits so the commits array matches the behind count and reflects only new upstream commits.
Human-readable output for the "Upstream commits" section incorrectly used recent_commits, causing it to show recent commits instead of actual upstream commits. Change the code to use upstream_commits for the human output so both JSON and human-readable views consistently show only true upstream commits.
Replace emoji-based output with ANSI colored text for clearer, more professional terminal display. Base branch names are shown in cyan; the upstream count is yellow for >0 and green for 0. Commit SHAs are yellow with descriptions dimmed. Status indicators are now textual tags with color: [ok] green, [integrated] blue, [conflict] red/yellow, and [empty] dimmed. Headers are bold and labels dimmed for a cleaner visual hierarchy. This improves readability and removes distracting emoji icons.
schacon
force-pushed
the
sc-branch-78
branch
from
6744278 to
4842d64
Compare
Move JSON serde structs into a dedicated json.rs module to separate concerns and improve organization. The JSON output structs (BaseCheckOutput, BaseBranchInfo, UpstreamInfo, UpstreamCommit, BranchStatusInfo) were moved and made pub(super) so mod.rs can access them. The old base.rs file was removed.
schacon
force-pushed
the
sc-branch-78
branch
from
4842d64 to
1ab0162
Compare
| "❗️ There are uncommitted changes in the worktree that may conflict with | ||
| the updates. Please commit or stash them and try again." |
There was a problem hiding this comment.
The string literal is broken across lines with incorrect indentation. The second line has extra indentation that will be included in the output string. Consider using a single line or properly formatted multi-line string without the leading whitespace.
| "❗️ There are uncommitted changes in the worktree that may conflict with | |
| the updates. Please commit or stash them and try again." | |
| "❗️ There are uncommitted changes in the worktree that may conflict with the updates. Please commit or stash them and try again." |
| writeln!(out, "✅ Everything is up to date")?; | ||
| } | ||
| None | ||
| } | ||
| UpdatesRequired { | ||
| worktree_conflicts, | ||
| statuses, | ||
| } => { | ||
| if !worktree_conflicts.is_empty() { | ||
| if let Some(out) = out.for_human() { | ||
| writeln!( | ||
| out, | ||
| "❗️ There are uncommitted changes in the worktree that may conflict with | ||
| the updates. Please commit or stash them and try again." | ||
| )?; | ||
| } | ||
| None | ||
| } else { | ||
| if let Some(out) = out.for_human() { | ||
| writeln!(out, "🔄 Updating branches...")?; |
There was a problem hiding this comment.
The Update subcommand still uses emoji-based output (✅, ❗️, 🔄) while the Check subcommand has been updated to use ANSI colored text. For consistency with the PR's stated goal of preferring "simpler, ansi colored style" over "emoji laden" style, consider updating the Update subcommand's output to match the new Check subcommand style.
This is an update to the
but base checkcommand. Previously,--jsonwas a no-op, this actually implements it. It also previously showed recent commits rather than upstream commits. Finally, I updated the style to match the simpler, ansi colored style preferred by the rest of the commands, rather than the emoji laden on this command used.If
--jsonis specified, it now works properly: